Add trusted-server-adapter-axum native dev server (PR 16)#643
Open
prk-Jr wants to merge 43 commits into
Open
Conversation
Move trusted-server-adapter-axum from workspace exclude to members list. Remove the global `target = "wasm32-wasip1"` build override from .cargo/config.toml (which forced the axum crate out of the workspace) and pass --target wasm32-wasip1 explicitly only for Fastly CI commands. Delete the now-redundant crate-local .cargo/config.toml. Update CI test-rust job to exclude the axum crate and pass the explicit target; test-axum job runs from the workspace root with -p flag. Add .edgezero/ to .gitignore to exclude the local KV store file.
Register AxumDevServer alongside FastlyViceroy in RUNTIME_ENVIRONMENTS so the full framework x runtime scenario matrix (WordPress, Next.js) runs against both platforms. AxumDevServer spawns the native trusted-server-axum binary (no WASM or Viceroy), binds to the fixed port 8787 (baked into axum.toml at compile time), and polls for any HTTP response as readiness (root returns 403 in test env). Binary path defaults to target/debug/trusted-server-axum, overridable via AXUM_BINARY_PATH. Settings are baked in at build time via TRUSTED_SERVER__* env vars, same as Fastly. The integration-tests.sh script now builds both the WASM and the native Axum binary with test-specific overrides (origin=127.0.0.1:8888). Add test_wordpress_axum and test_nextjs_axum individual test functions. Ignore .edgezero/ at workspace root (local KV store from the dev server).
- README: update Quick Start and Development commands for both runtimes - getting-started: add Axum as Option A (no Fastly CLI needed) - architecture: add trusted-server-adapter-axum to Core Components; add Runtime Targets table - testing: fix cargo test commands; add Axum adapter section; split Local Server Testing into Axum vs Fastly; restore clippy step in CI/CD workflow example alongside new test-axum job
The integration test matrix includes AxumDevServer which requires the native trusted-server-axum binary. Add build-axum step to the shared setup action, package the binary alongside the WASM artifact, and pass AXUM_BINARY_PATH to the integration test run step.
prk-Jr
requested review from
ChristianPavilonis and
aram356
and removed request for
aram356
aram356
requested changes
Apr 20, 2026
aram356
left a comment
aram356
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Introduces a native Axum dev-server adapter that runs the full trusted-server pipeline locally without Fastly Compute or Viceroy, promotes the crate to a workspace member, and extends the integration-test matrix to cover both runtimes. Requesting changes for a handful of small but concrete issues: unused deps, stale lockfile, doc/code drift, and a middleware bypass on the startup-error path.
Blocking
🔧 wrench
- Unused direct dependencies:
serde_jsonandtrusted-server-jsdeclared incrates/trusted-server-adapter-axum/Cargo.tomlbut never referenced insrc/ortests/. See inline. - Stale crate-local
Cargo.lock: ~100 KB file that cargo now silently ignores (crate is a workspace member). Delete it to avoid lockfile drift. CLAUDE.mdcomment contradicts the PR: says "excluded from workspace" at line 14 while the PR makes it a workspace member. See inline.- Log text references a non-existent
NoopKvStore: the code usesUnavailableKvStore(src/platform.rs:367vs :379). See inline. - Startup-error router bypasses middleware:
startup_error_router()has noFinalizeResponseMiddleware/AuthMiddleware, breaking the "every response has X-Geo-Info-Available" invariant and bypassing operator headers + basic auth on startup-failure responses. See inline onsrc/app.rs:134.
Non-blocking
🤔 thinking
send_async/selectdiverges from Fastly (eager execution changes error-surface timing and ordering). Trade-off is documented, but consider a breadcrumb log or realtokio::spawnfan-out. See inline.Body::Streamoutbound body silently truncated to empty on axum. See inline.- Env-var namespace collisions possible due to
-/./_normalization. See inline. - Fixed port 8787 baked at compile time can TIME_WAIT-flake across sequential integration tests. See inline.
♻️ refactor
reqwest::Clientrebuilt per request — defeats connection pool. Move to sharedArc<AxumPlatformHttpClient>inAppState. See inline.- Env-var tests not isolated — use
temp-env(already a workspace dep). See inline. bytes::Bytes→Vec<u8>round-trip on request/response bodies is wasted allocation. See inline.tests/routes.rsuses.unwrap()instead of.expect("should ...")— CLAUDE.md applies to test code. See inline.
🌱 seedling
- No
/healthendpoint —AxumDevServer::health_check_path()returns/healthbut the app never registers it; tests work around withwait_for_any_response. See inline.
⛏ nitpick
- Crate-local
.gitignoreis redundant with the workspace.gitignore. See inline. build_per_request_servicesis a no-op wrapper aroundbuild_runtime_services. See inline.
📝 note
test-axumCI job runs withoutTRUSTED_SERVER__*env vars — every request goes through the startup-error path. Smoke tests still pass, but not a great signal. See inline.
CI Status
- fmt: PASS (verified locally,
cargo fmt --all -- --check) - clippy (axum crate, host target): PASS with zero warnings
cargo test -p trusted-server-adapter-axum: 18 tests PASS- GitHub CI on
75fe0d01: prepare artifacts + integration tests + browser tests all PASS
… bypass, refactors
Blocking:
- Remove unused serde_json and trusted-server-js from Cargo.toml
- Delete crate-local Cargo.lock (now silently ignored as workspace member)
- Fix CLAUDE.md workspace layout comment (drop "excluded from workspace")
- Fix log message naming NoopKvStore → UnavailableKvStore in build_runtime_services
- Wrap startup_error_router with FinalizeResponseMiddleware(Settings::default()) so
startup-error responses carry X-Geo-Info-Available and operator response_headers
Refactor:
- Move AxumPlatformHttpClient to AppState; share Arc across requests via
build_runtime_services(ctx, Arc::clone(&state.http_client)) to preserve
the reqwest connection pool
- Remove build_per_request_services no-op wrapper; inline at call sites
- Use temp_env::with_var in config/secret store tests for proper isolation
- Replace .unwrap() with .expect("should ...") throughout test code per CLAUDE.md
Nitpick:
- Delete redundant crate-local .gitignore (covered by workspace .gitignore)
…ce breadcrumbs Port fix: - main.rs reads PORT env var at startup; when set, uses AxumDevServer::with_config with a dynamic SocketAddr instead of the run_app default (hardcoded 8787) - Integration test spawner (axum.rs) now calls find_available_port() and passes PORT=<port> to the child process, matching the Fastly env's dynamic-port pattern - Fallback to AXUM_DEFAULT_PORT=8787 if find_available_port fails (offline runner) Divergence breadcrumbs: - send_async: debug log noting that execution is eager and errors surface immediately, not at select() time as they do on Fastly - select: debug log noting that index 0 is popped unconditionally (sequential, not first-to-complete) — any fan-out ordering tests should use the Fastly runtime
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Nice addition overall — the Axum adapter is headed in a useful direction. I found a few correctness gaps in the platform shim plus a couple of documentation mismatches around what the Axum runtime can currently support.
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Reviewed the Axum native dev server PR. I found a couple of Axum/Fastly parity issues worth addressing and several mixed-target workspace documentation updates. CI checks visible from gh pr checks are passing.
Body-only findings
Proxy response finalization repeats metadata and image heuristics
- Severity: P2
- File:
crates/trusted-server-core/src/proxy.rs - Line: 200-405
- Description:
finalize_proxied_response()andfinalize_proxied_response_streaming()duplicate response metadata extraction/logging and the image/pixel heuristic block. The only meaningful differences are content-encoding processing and the streaming log prefix, so changes to image handling or origin-response logging now require parallel edits. - Suggestion: Extract small helpers such as
origin_response_metadata(req, &response)andapply_image_passthrough_metadata(req, target_url, &ct, &mut response, log_prefix), then keep only the HTML/CSS processing branch in the buffered finalizer.
Related stale-build-doc locations that may not be commentable inline: CLAUDE.md:42-43 also still shows bare cargo build.
- Disable reqwest auto-redirects in AxumPlatformHttpClient so core proxy code receives 3xx responses and applies its own allowed_domains policy - Map select() task panics and per-request errors into ready: Err(...) so the auction orchestrator logs provider failures and continues rather than treating one bad provider as a fatal auction error - Extract buffer_publisher_response into trusted_server_core::publisher; both adapters now call it, removing duplicated buffering logic - Extract origin_response_metadata and apply_image_passthrough_metadata helpers in proxy.rs; both finalizers share one copy of metadata extraction and pixel heuristics - Rewrite app.rs with NamedRouteHandler enum, execute_handler, and a named_routes table mirroring the Fastly adapter; remove per-route boilerplate closures (~180 lines) - Add crate-level and per-module doc comments to axum adapter lib.rs - Replace bare cargo build/test/clippy docs with target-specific aliases across README, getting-started, and testing guides - Document PORT env var override for Axum dev server
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
Review Summary
The current branch is in good validation shape (fmt, clippy-fastly, clippy-axum, test-fastly, and test-axum were reported passing), but I found two Fastly EdgeZero parity regressions plus one stale documentation/template issue. One finding is inline below; the other findings are included here because the referenced lines are outside the current PR diff and cannot be attached inline.
High (P1)
-
Fastly EdgeZero path loses HTTPS scheme metadata —
crates/trusted-server-adapter-fastly/src/app.rs:164build_per_request_services()setstls_protocol/tls_ciphertoNone, andedgezero_main()strips spoofable forwarded headers before dispatch. That meansRequestInfo::from_request()can fall back tohttpfor Fastly EdgeZero publisher requests, so HTTPS pages may get rewritten URLs emitted ashttp://..., causing mixed-content/security regressions.Suggested fix: preserve trusted TLS metadata before dispatch, e.g. extend the Fastly EdgeZero request context or inject a trusted internal scheme signal after sanitizing client headers.
Low (P3)
-
Active docs/templates still reference obsolete workspace commands —
docs/guide/integration-guide.md:259,.github/pull_request_template.md:26The branch updates most commands to
cargo test-fastly/cargo test-axumand split clippy aliases, but the integration guide and PR template still instruct contributors to runcargo test --workspace/cargo clippy --workspace....Suggested fix: update these to the new target-specific aliases.
Fix EdgeZero HTTPS scheme detection regression: capture tls_protocol and tls_cipher from the raw FastlyRequest before dispatch_with_config_handle consumes it, inject as trusted internal headers x-ts-tls-protocol / x-ts-tls-cipher (added to INTERNAL_HEADERS), and read them in build_per_request_services to populate ClientInfo. This restores parity with the legacy path where RequestInfo::from_request detects HTTPS via ClientInfo TLS fields rather than falling back to the default "http". Skip the from_fastly_response -> to_fastly_response round-trip for middleware-finalized responses in edgezero_main. Checking x-ts-finalized on the FastlyResponse directly and calling send_to_client() without the intermediate take_body_bytes() call avoids re-materializing the full response body in WASM heap on the normal registered-route path. Update integration-guide.md and pull_request_template.md to use the target-specific cargo aliases (cargo test-fastly / cargo test-axum, cargo clippy-fastly / cargo clippy-axum) instead of the removed --workspace variants.
aram356
requested changes
May 28, 2026
aram356
left a comment
aram356
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Re-review of PR16 (Axum dev-server adapter) at head a6e1073b. Most prior findings from earlier reviews are resolved; CI is green. This re-review finds one new blocking security issue introduced by the most recent commit (TLS-header trust contract not enforced) plus several non-blocking concerns.
Note: PR base is
feature/edgezero-pr15-remove-fastly-core, notmain. PR15 must merge first; ordering should be confirmed before merge of this PR.
Blocking
🔧 wrench
- Spoofable
x-ts-tls-protocol/x-ts-tls-cipheron the EdgeZero entry path — see inline at crates/trusted-server-adapter-fastly/src/main.rs:178. The trust contract documented at L173-176 is not enforced; the spoofable-headers strip list does not includex-ts-tls-*, and the trusted-sourceset_headeris conditional on the Fastly SDK returningSome. Consequence:detect_request_schemecan be spoofed tohttpsover plain HTTP, affecting cookieSecure, URL rewriting, and any scheme-gated handler.
Non-blocking
🤔 thinking
reqwest::Clientrebuilt per request regresses a previously-resolved finding — see inline at crates/trusted-server-adapter-axum/src/platform.rs:503.health_check_path()returns/healthbut no route is registered — see inline at crates/integration-tests/tests/environments/axum.rs:70.
♻️ refactor
- Stateless platform shims allocated
Arc::new(...)per request — see inline at platform.rs:497. - Two divergent code paths in
main.rs::main()— see inline at main.rs:16.
⛏ nitpick
- Logger init failure silently swallowed in
main()— see inline at main.rs:9. PORTparse error falls back instead of failing fast — see inline at main.rs:43.
🌱 seedling
- Operator-header validation is per-request — see inline at middleware.rs:105.
📌 out of scope
- Consolidate known dev-server limitations in
crates/trusted-server-adapter-axum/src/lib.rs. The PR author's responses to prior reviews acknowledge several intentional Axum-vs-Fastly divergences (eagersend_async, sequentialselectordering, truncatedBody::Streamoutbound, no KV/geo, env-var key collision surface). Today they're scattered acrossplatform.rs,app.rs, andmiddleware.rsdoc comments — a single "Known limitations" section inlib.rswould make them discoverable. Worth a follow-up issue rather than this PR.
CI Status
- fmt: PASS
- clippy (fastly + axum): PASS
- rust tests (Fastly via Viceroy + Axum native): PASS
- integration tests: PASS
- browser integration tests: PASS
All 3 PR checks green at head a6e1073b.
Blocking: - Strip x-ts-tls-protocol and x-ts-tls-cipher from the inbound request before conditionally re-setting them from the Fastly SDK in edgezero_main; without this, a plain-HTTP client injecting X-TS-TLS-Protocol spoofs the scheme detection path, affecting cookie Secure and URL rewriting - Add regression test in app.rs asserting tls_protocol/tls_cipher are None when the trusted headers are absent after the strip+set logic Non-blocking: - Register GET /health -> 200 ok on TrustedServerApp so health_check_path() is satisfied and the shared wait_for_ready helper can be used - Promote stateless shims (AxumPlatformConfigStore, AxumPlatformSecretStore, UnavailableKvStore, AxumPlatformBackend, AxumPlatformGeo) to OnceLock statics in build_runtime_services so Arc::clone is used per request instead of allocating a fresh Arc each time - Unify the two divergent main() branches into a single code path using AxumDevServer::with_config; log logger init failure with eprintln! instead of silently swallowing it - Exit non-zero on PORT parse error instead of falling back silently to axum.toml default — the fallback surprises tooling expecting the server at the explicitly requested port
Collaborator
Author
|
All round-2 findings resolved in commit 746993c. Blocking — fixed
Non-blocking — addressed
Deferred
CI: |
Conflicts resolved: - app.rs (2 doc conflicts): PR16 wins on the module-level and build_per_request_services docs — they correctly describe the x-ts-tls-* header injection mechanism added in this PR - app.rs (test import): merge both — PR16's build_per_request_services and PR15's build_state_from_settings both needed in the test module - main.rs (edgezero_main finalization): PR15 wins — take_finalize_sentinel + inlined resolve_geo_for_response + apply_finalize_headers replaces the older apply_entry_point_finalize approach - main.rs (function): PR15 wins — take_finalize_sentinel function replaces apply_entry_point_finalize - main.rs (BoundedWriter + resolve_publisher_response_buffered): PR15 wins — add these structures needed for the EdgeZero buffered publisher path - main.rs (test): PR15 wins — add take_finalize_sentinel_strips_sentinel test - publisher.rs (doc): PR16 wins — buffer_publisher_response description is correct for that function; PR15 doc was misattributed to stream_publisher_body
The merge conflict resolution took PR15's take_finalize_sentinel code expecting an HttpResponse but left the fastly_response variable unconverted. Add the missing let mut response = compat::from_fastly_response(fastly_response) before the sentinel check.
- Remove BoundedWriter and resolve_publisher_response_buffered from main.rs; they were copied in by the merge but belong in app.rs for the EdgeZero path; the legacy path uses stream_publisher_body directly - Remove the now-unused std::io::Write import - Fix let mut fastly_response to let fastly_response (no longer mutated)
aram356
requested changes
Jun 3, 2026
aram356
left a comment
aram356
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Round-4 re-review of the Axum dev-server adapter, scoped to the PR's own diff against its base feature/edgezero-pr15-remove-fastly-core. All blocking findings from the previous three rounds are resolved (deps, lockfile, middleware bypass, fan-out semantics, Body::Stream buffering, env-var rename, fmt/clippy failures, CLAUDE.md/AGENTS.md/agent-doc drift, wasm clippy coverage, fixed-port flakiness, /health registration, handler boilerplate). Verified locally: cargo fmt --all -- --check passes, clippy -p trusted-server-adapter-axum is clean, and the 19 axum tests pass.
Requesting changes for one regression introduced by this PR: the removal of the publisher buffered-body size cap.
Blocking
🔧 wrench
- Unbounded publisher buffer / dead
max_buffered_body_bytes: deletingresolve_publisher_response_buffered(and itsBoundedWriter) in favor of the new unboundedbuffer_publisher_responsedropssettings.publisher.max_buffered_body_bytesenforcement on the Fastly EdgeZero path. The setting is still defined, defaulted to 16 MiB, and documented as preventing Wasm-heap OOM, but is now read nowhere. See inline oncrates/trusted-server-core/src/publisher.rs:418.
Non-blocking
📌 out of scope
- Per-request
reqwest::Client(crates/trusted-server-adapter-axum/src/platform.rs:521): the no-pooling revert is deliberate, but its comment cites a Next.js "poisoned connection after a truncated POST." The empty-body branch (if !body_bytes.is_empty()) skippingContent-Length: 0is a likely latent cause worth a follow-up issue rather than masking with per-request clients.
📝 note
- Weak happy-path coverage: the
test-axumCI job runs withoutTRUSTED_SERVER__*env vars andtests/routes.rsonly asserts non-404/non-5xx, so requests flow throughstartup_error_routerand 200/302 paths are never exercised. Pre-existing; deferred.
⛏ nitpick
Cargo.toml:83:reqwestadded out of alphabetical order (sits beforeregex).crates/trusted-server-adapter-axum/Cargo.toml:33-35:edgezero-adapter-axum,edgezero-core, andreqwestare re-declared in[dev-dependencies]though already normal dependencies with no added features — redundant.
CI Status (verified locally on macOS host)
- fmt: PASS (
cargo fmt --all -- --check) - clippy (axum, native): PASS (zero warnings)
cargo test -p trusted-server-adapter-axum: PASS (19 tests)- GitHub CI (integration tests, browser integration tests): PASS
- Note:
Run Tests/Run Formatworkflows only trigger on PRs tomain, so fmt/clippy/test-fastly/test-axum have not run on this PR's feature-branch target; they were verified locally instead.
ChristianPavilonis
requested changes
Jun 3, 2026
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
Automated review by Yesman.
I found two blocking issues that should be fixed before merge: the Axum async select path can mislabel remaining backend requests after the first completion, and the shared publisher buffering helper drops the configured max-buffer guard that protected the Fastly EdgeZero path from unbounded buffering. Inline comments include details.
Pick up PR15 commits since the first merge (cedea8b — remove stale fastly::Body reference from platform/mod.rs docs). Conflicts resolved: - Cargo.lock: auto-resolved, take union - integration-tests/tests/integration.rs: keep both Axum tests (PR16) and EC lifecycle test (PR15) - trusted-server-adapter-fastly/src/main.rs: keep PR16 TLS strip/inject headers AND PR15 into_core_request + oneshot dispatch Post-merge compilation fixes for trusted-server-adapter-axum: - app.rs: update handle_proxy call to use ProxyDispatchInput struct (PR15 bundled 5 args into struct to stay within 7-arg cap) - app.rs: update handle_auction call with new kv/registry/ec_context args added in PR15 (pass None/default for Axum dev server) - platform.rs: add failed_backend_name: None to PlatformSelectResult initializer (new field added in PR15)
The PR15 merge reintroduced fastly into trusted-server-core through ec/kv.rs, ec/rate_limiter.rs, and an orphaned backend.rs, breaking native linking for the Axum dev server. - Delete orphaned core backend.rs (adapter already owns its copy) - Keep the RateLimiter trait in core; move FastlyRateLimiter and RATE_COUNTER_NAME to the Fastly adapter - Add the EcKvStore primitive trait (lookup with generation marker, conditional insert, prefix count, delete) in ec/kv_backend.rs - Rewrite KvIdentityGraph on top of EcKvStore so CAS retry loops, tombstone semantics, and validation stay in core - Implement FastlyEcKvStore in the adapter mapping onto the Fastly KV Store API - Add in-memory and failing test backends; cover create/revive, upsert, and tombstone paths natively
get_settings() now rejects the placeholder secrets baked into trusted-server.toml (hardened in PR14/15) instead of only logging warnings, so adapter tests that loaded baked settings began failing. - Build test settings from explicit TOML with non-placeholder values in the Axum middleware tests and the Fastly app tests - Add TrustedServerApp::routes_with_settings() so the Axum route integration tests drive the real router without the baked settings - Accept the designed 501 Not Implemented for admin key routes on the dev server; only an unhandled 500 fails the test
- Restore the publisher buffered-body size cap: reinstate BoundedWriter (now in core so both adapters share it) and enforce settings.publisher.max_buffered_body_bytes in buffer_publisher_response, with unit tests covering the cap - Fix backend-name misattribution in AxumPlatformHttpClient::select: futures::future::select_all uses swap_remove and makes no ordering guarantee for remaining futures, so positional index reconstruction could pair later auction responses with the wrong bidder. Each AxumPendingHandle now implements Future and resolves to its own backend name - Reorder reqwest after regex in workspace Cargo.toml - Drop redundant dev-dependency re-declarations (edgezero-adapter-axum, edgezero-core, reqwest) in the Axum adapter manifest
Removing the fastly dependency from trusted-server-core dropped its transitive wasm dependencies from the integration-tests dependency graph, leaving the crate's standalone Cargo.lock stale and failing the --locked check in check-integration-dependency-versions.sh.
The 'Verify Fastly WASM release build' step builds for wasm32-wasip1 but the job's toolchain setup never installed that target, failing with: can't find crate for core.
aram356
requested changes
Jun 12, 2026
aram356
left a comment
aram356
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Re-review of the Axum native dev-server adapter against its base (feature/edgezero-pr15-remove-fastly-core). Most prior blocking items are now genuinely fixed (verified): the select_all success-path backend-name misattribution, the unbounded publisher buffer / dead max_buffered_body_bytes cap, TLS-header spoofing strip, /health route, PORT exit-on-error, the clippy split covering both targets, and the SYNTHETIC__SECRET_KEY → EDGE_COOKIE__SECRET_KEY rename. The new trait extractions (EcKvStore, RateLimiter) that remove fastly from core are well-executed and faithful to the original semantics.
Requesting changes for two items: a correctness regression on the auction error path (the counterpart of the select_all fix was left incomplete) and an incomplete migration of the workspace-command aliases into the live dev-tooling files.
Blocking
🔧 wrench
-
Axum auction: failed providers silently vanish —
crates/trusted-server-adapter-axum/src/platform.rs:484returnsfailed_backend_name: Noneon the error path, so a failed provider is never attributed and noBidStatus::Erroris recorded (it disappears via the orchestrator's "backend not identified" branch). The Fastly adapter returnsSome(failed_name). Cross-platform parity regression. See inline. -
Live dev-tooling commands broken by the global-target removal — removing
[build] target = wasm32-wasip1plus making axum a workspace member means barecargo {build,test,clippy} --workspacenow compiles the wasm-onlyfastlycrate for the host and fails.CLAUDE.md,AGENTS.md,README.md, and three agent docs were migrated to the*-fastly/*-axumaliases, but four live tooling files were missed (same root cause):.claude/commands/check-ci.md:4-5— the/check-cicommand (cargo clippy/test --workspace).claude/commands/verify.md:3,6,7— the/verifycommand (cargo build/clippy/test --workspace).claude/commands/test-all.md:4— the/test-allcommand (cargo test --workspace).claude/agents/build-validator.md:14,26(cargo build/clippy --workspace)
A contributor running
/check-cior/verifynow hits the exact failureAGENTS.mdwarns against. Update each tocargo test-fastly && cargo test-axum/cargo clippy-fastly && cargo clippy-axum.
Non-blocking
🤔 thinking
- Dropped
remainingauction handles detach, not abort —platform.rs:453; dev-only, up to 30s of wasted background work per dropped bidder. See inline. - Unbounded upstream response buffering —
platform.rs:348(andsend_asyncat ~413); no cap, no log on the response side. See inline. - Redundant double error-wrapping in KV paths —
ec/kv.rs:663and:762re-wrap with the same message the backend already attached. See inline.
♻️ refactor
- Dead
ifover discarded bool —proxy.rs:370;apply_image_passthrough_metadatareturns aboolboth call sites ignore. See inline.
🌱 seedling
- CAS-conflict retry branches untested —
ec/kv.rs:331; no test injects a generation conflict, so the retry / concurrent-revive / exhaustion paths are uncovered. See inline. - Per-request
reqwest::Clientkeeps the pool disabled —platform.rs:280; documented Next.js-regression workaround, follow-up issue acknowledged. See inline.
⛏ nitpick
- Bare version strings for dev-deps —
crates/trusted-server-adapter-axum/Cargo.toml:31,34(axum/tower) vs the workspace-pinning convention. See inline. - Weak route-test assertions / no auction fan-out test —
tests/routes.rs:118. See inline.
📝 note
- Misplaced WASM-verify step —
.github/workflows/test.yml:77; lives intest-axum, only compiles becauserust-toolchain.tomlpins the target. See inline. - CI never exercised fmt/clippy/test on this PR —
test.ymlandformat.ymlare gatedpull_request: branches: [main]; this PR targets a feature branch, so onlyintegration-tests.ymlran (the green checks). The fmt/clippy/test-fastly/test-axum gates will run for the first time on retarget tomain. CLAUDE.mdbarecargo checkunder Testing & Quality now fails at the workspace root; scope it with-por an alias.
CI Status
- integration tests / browser tests / prepare artifacts: PASS (on
4a2e1134) - fmt / clippy / test-fastly / test-axum: not run on this PR (workflows gated to PRs targeting
main); clippy-split config verified correct locally
Reconciles PR15's EC parity and review fixes with PR16's trait extractions, preferring the PR16 implementation where both sides changed the same code: - maybe_identity_graph / require_identity_graph keep PR16's KvIdentityGraph::new(FastlyEcKvStore::new(...)) construction (now pub(crate) for app.rs), and app.rs EC dispatch uses the core buffer_publisher_response instead of re-adding the adapter-local BoundedWriter copy that PR16 had already moved into core - core buffer_publisher_response adapted to the plain usize max_buffered_body_bytes from PR15 (the unbounded None arm is unrepresentable) - EcFinalizeState carries a use_finalize_kv flag instead of the KvIdentityGraph itself: the graph now wraps a non-Sync dyn EcKvStore and cannot ride in response extensions, so edgezero_main rebuilds it from settings when the flag is set - FastlyRateLimiter import moved to the adapter-local module per PR16's rate-limiter extraction - Test helpers merged: PR16's TLS-header tests and minimal_state plus PR15's test_router and absolute-URI empty_request - integration-tests lock keeps PR16's post-fastly-removal graph; shared dependency versions verified with the check script
Blocking:
- Attribute failed auction providers on the Axum select error path:
failed_backend_name now carries the backend whose request failed,
matching the Fastly adapter, so the orchestrator removes the provider
and records BidStatus::Error instead of dropping it through the
"backend not identified" branch. Adds a select-level regression test
(request to a closed port) asserting the attribution
- Migrate the remaining live dev-tooling files to the per-target
aliases: /check-ci, /verify, /test-all, and the build-validator agent
now use cargo {build,clippy,test}-{fastly,axum}. New build-* and
check-* aliases added to .cargo/config.toml; CLAUDE.md's bare
cargo build / cargo check examples updated to the aliases
Non-blocking:
- Abort dropped auction tasks: AxumPendingHandle aborts its JoinHandle
on drop so deadline-dropped bidders stop instead of running up to the
30s transport timeout in the background
- Log buffered upstream response sizes on both the send and send_async
paths so a large upstream is visible in dev instead of silently
growing the heap
- Drop the redundant double error-wrapping in count_hash_prefix_keys
and delete — the backend already attaches store context
- apply_image_passthrough_metadata returns () instead of a bool both
call sites discarded
- axum and tower dev-dependencies moved to workspace pins
- Admin route tests assert the fixed 501 contract instead of != 404,
and CAS-conflict paths are now covered: a conflict-injecting EcKvStore
test backend exercises the retry-then-succeed, concurrent-revive
short-circuit, and retry-exhaustion arms of create_or_revive
Collaborator
Author
|
Re the review-summary items without inline threads (all in f7e8846 unless noted):
|
ChristianPavilonis
approved these changes
Jun 12, 2026
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
I completed the review and found one low-severity gap (P3, behavior parity) plus previously validated no P0/P1/P2 blockers.
- [P3] (named_routes): routes are not mirrored in Axum (, , and related canonical variants). These routes currently route into generic fallback behavior rather than the handler logic used on Fastly.
Recommendation: consider adding canonical route entries in Axum’s route table (e.g., as aliases with same handlers), so production parity remains consistent and consumers hitting documented API paths get expected responses even in dev/runtime testing mode.
ChristianPavilonis
approved these changes
Jun 12, 2026
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
I completed the review and found one low-severity gap (P3, behavior parity) plus previously validated no P0/P1/P2 blockers.
- [P3] crates/trusted-server-adapter-axum/src/app.rs (named_routes):
_tsroutes are not mirrored in Axum (/_ts/admin/keys/*,/_ts/api/v1/identify,/_ts/api/v1/batch-syncand related canonical variants). These routes currently route into generic fallback behavior rather than the handler logic used on Fastly.
Recommendation: add _ts canonical route entries in Axum’s route table (as aliases to existing handlers where behavior is shared), so production-like API paths remain consistent in Axum test/runtime mode.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
trusted-server-adapter-axumas a native (non-WASM) dev server so the full trusted-server pipeline can be run and tested locally without Fastly Compute or Viceroytarget = "wasm32-wasip1"override from.cargo/config.toml; Fastly-specific commands now pass--target wasm32-wasip1explicitlyChanges
crates/trusted-server-adapter-axum/src/platform.rsPlatformConfigStore,PlatformSecretStore,PlatformBackend,PlatformGeo,PlatformHttpClient— env-var-backed implementationscrates/trusted-server-adapter-axum/src/middleware.rsFinalizeResponseMiddleware+AuthMiddleware— mirrors Fastly adapter, always emitsX-Geo-Info-Available: falsecrates/trusted-server-adapter-axum/src/app.rsTrustedServerAppimplementingHookswith all 11 routes wiredcrates/trusted-server-adapter-axum/src/main.rs+axum.tomlcrates/trusted-server-adapter-axum/tests/routes.rsEdgeZeroAxumService(no live TCP server)crates/integration-tests/tests/environments/axum.rsAxumDevServerruntime environment added to the matrixcrates/integration-tests/tests/environments/mod.rsAxumDevServeralongsideFastlyViceroycrates/integration-tests/tests/integration.rstest_wordpress_axum+test_nextjs_axumindividual test functionsscripts/integration-tests.sh.cargo/config.tomltarget = "wasm32-wasip1"; keep only the viceroy runnerCargo.tomltrusted-server-adapter-axumfrom[exclude]to[members]crates/trusted-server-adapter-axum/Cargo.toml.github/workflows/test.ymltest-axumCI job;test-rustnow passes--target wasm32-wasip1explicitlyCLAUDE.md.gitignore(root + adapter).edgezero/(local KV store created by dev server)Closes
Closes #497
Test plan
cargo test --workspace(Fastly/WASM crates via Viceroy)cargo test -p trusted-server-adapter-axum(8 route + middleware tests)cargo clippy --workspace --all-targets --all-features -- -D warningscargo fmt --all -- --checkcd crates/js/lib && npx vitest run(282 tests)test_wordpress_fastly,test_nextjs_fastly,test_wordpress_axum,test_nextjs_axumall passcargo run -p trusted-server-adapter-axumstarts on port 8787Checklist
unwrap()in production code — useexpect("should ...")logmacros (notprintln!)